Arduino service #812

billonalex · 2020-05-20T15:37:59Z

Pull Request check-list

To ensure your Pull Request can be accepted as fast as possible, make sure to review and check all of these items:

If your changes affects code, did your write the tests?
Are tests passing? (npm test on both front/server)
Is the linter passing? (npm run eslint on both front/server)
Did you run prettier? (npm run prettier on both front/server)
If you are adding a new features/services, did you run integration comparator? (npm run compare-translations on front)
Did you add fake requests data for the demo mode (front/src/config/demo.json) so that the demo website is working without a backend? (if needed) See https://demo.gladysassistant.com.
If your changes modify the API (REST or Node.js), did you modify the API documentation? (Documentation is based on comments in code)
If you are adding a new features/services which needs explanation, did you modify the user documentation? See the GitHub repo and the website.

NOTE: these things are not required to open a PR and can be done afterwards / while the PR is open.

Description of change

Please provide a description of the change here. It's always best with screenshots, so don't hesitate to add some!

This PR is about the Arduino service. Gladys will now be able to interact with Arduino card. The user uploads the generic code on the card, setups the service, creates the device, and interacts with hardware.

codecov · 2020-05-27T06:39:21Z

Codecov Report

Merging #812 (f7a4b01) into master (3febb17) will decrease coverage by 0.04%.
The diff coverage is 92.92%.

@@            Coverage Diff             @@
##           master     #812      +/-   ##
==========================================
- Coverage   94.52%   94.47%   -0.05%     
==========================================
  Files         485      497      +12     
  Lines        6444     6642     +198     
==========================================
+ Hits         6091     6275     +184     
- Misses        353      367      +14

Impacted Files	Coverage Δ
server/lib/device/light/light.getLightsInRoom.js	`100.00% <ø> (ø)`
server/services/usb/api/usb.controller.js	`100.00% <ø> (ø)`
server/services/arduino/lib/listen.js	`80.00% <80.00%> (ø)`
server/services/arduino/lib/init.js	`84.00% <84.00%> (ø)`
server/services/arduino/lib/setup.js	`84.61% <84.61%> (ø)`
server/services/arduino/lib/send.js	`90.90% <90.90%> (ø)`
server/services/arduino/lib/device.setValue.js	`94.44% <94.44%> (ø)`
server/services/arduino/lib/onPortData.js	`94.73% <94.73%> (ø)`
server/services/arduino/api/arduino.controller.js	`100.00% <100.00%> (ø)`
server/services/arduino/index.js	`100.00% <100.00%> (ø)`
... and 18 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3febb17...f7a4b01. Read the comment docs.

VonOx · 2020-05-27T06:47:49Z

Hi @billonalex , how did you sync your fork from master ?

billonalex · 2020-05-27T06:52:56Z

Hi @VonOx ,

To merge from the original repo, I used :

git remote add upstream https://github.com/GladysAssistant/Gladys.git

and

git fetch --all
git merge upstream/master
git push

each time a new commit appeared in GladysAssistant repo.

VonOx · 2020-05-27T07:01:06Z

Ok its fine ! Something weird with my github ( and me :-) )
I saw philips-hue stuff in PR related to arduino , so I was confused.

Nice job btw

billonalex · 2020-05-27T07:05:50Z

Yeah, when I ran prettier, the "tests/services/philips-hue/lights.json" changed, which caused bugs during tests ! So I manually corrected the file by cancelling changes prettier made on this file and discarding when running prettier after. Technically the file is now the same as it used to be 🙂

Pierre-Gilles · 2020-06-02T08:36:24Z

Thanks for your PR !

I agree with @VonOx, @billonalex I'm not sure you're merging your fork with master the right way, I see some files from other PRs affected here, and sometimes it's conflicting with the master branch:

billonalex · 2020-06-02T09:00:38Z

Hi @Pierre-Gilles,

I just merged your latest commits. There shouldn't be conflict anymore.

However, I don't understand how could I merge my fork with master the right way ? I do the exact step as described upper each time I have to merge changes.

Pierre-Gilles · 2020-06-02T10:01:04Z

This time it seems good! You maybe did a mistake previously :)

About codecov not passing, the diff coverage is available here:

https://codecov.io/gh/GladysAssistant/Gladys/compare/354d2f6e974f4bc3b95d9139059b026ca5d66275...e239755d0c979fef0a7257aeab0c6611e353b5c8/diff

billonalex · 2020-06-02T10:15:18Z

Perfect :)

However, I'm a little confused about codecov... If some changes have to be made could you please give me some directions ?

Thank you

Pierre-Gilles · 2020-06-02T14:44:48Z

Codecov indicates the lines not covered by tests ! To fix codecov, you need to write tests so that every lines indicated by codecov are covered

Example:

Here, it seems that the configure function is not fully covered in tests

Does it make sense? :)

Pierre-Gilles

Thanks for this very great PR ! 👏

This is rare to have such quality in a first shot, so congrats, you did something really great :)

I just read the server code for now, and I have a few feedbacks, mostly on error handling.

While it's great the catch errors, something errors should be thrown and sent back to the frontend so the frontend knows that something is wrong.

In Gladys 4, the user will never see the logs of Gladys 4, so the errors should be visible in the UI !

Pierre-Gilles · 2020-06-08T09:49:38Z

server/services/arduino/api/arduino.controller.js

+   * @apiGroup Arduino
+   */
+  async function setup(req, res) {
+    arduinoManager.setup(req.body);


From what I saw, this function never rejects any errors

How is the user going to know when something went wrong?

Error handling is very important and you shouldn't res.json({ success: true}); all the time. If something is wrong, reject a nice error message and in the UI display an explicit error message

I modified setup.js script to return a json { success: true} or { success: false} to the front.

This is not what I said! You should reject errors, not resolves with { success: false}. When something is wrong, the HTTP code should not be "200 ok" but should be relative to the error.

This is an example in the Philips Hue library : https://github.com/GladysAssistant/Gladys/blob/master/server/services/philips-hue/lib/light/light.setValue.js

When something is wrong, we throw a clear error which will results in a "404" error, clear for the client.

We provide lots of errors in utils/coreErrors and utils/httpErrors. Let me know if you need more help :)

server/services/arduino/lib/device.configure.js

server/services/arduino/lib/device.setValue.js

billonalex · 2020-06-08T11:02:54Z

@Pierre-Gilles Thanks for your review 🙂

Changes are made. I run a prettier, local coverage and I push my changes !

billonalex · 2020-06-10T09:00:44Z

Hello @Pierre-Gilles

Did you have the time to look at changes that I made ? 🙂

Pierre-Gilles

Thanks for your changes! I added a feedback on the error handling :)

Btw, I'm waiting for people to test it on the forum before merging this.

Pierre-Gilles · 2020-11-27T12:30:38Z

Hi! Just wanted to know what's the status of this PR? :)

billonalex marked this pull request as ready for review May 25, 2020 17:53

billonalex changed the title ~~Arduino service~~ [WIP] Arduino service May 27, 2020

billonalex changed the title ~~[WIP] Arduino service~~ Arduino service May 27, 2020

billonalex requested a review from Pierre-Gilles June 7, 2020 21:25

Pierre-Gilles requested changes Jun 8, 2020

View reviewed changes

billonalex requested a review from Pierre-Gilles June 8, 2020 11:51

Pierre-Gilles requested changes Jun 16, 2020

View reviewed changes

billonalex added 10 commits December 2, 2020 08:41

Update UI

005ab29

Update logs in back

90521f2

Arduino code link

6919258

arduino code link

039dea0

Devices : Add pulse length

f6c1d2f

Correction

b14e416

Test on emit 433

b975ef9

Add pulse length for chacon

8cb3853

add updatePulseLength action

cfa95f8

test pulse length

998a3d3

billonalex added 27 commits December 2, 2020 08:57

[Debug] : ArduinoController --> Only one param

355209a

[Debug] : setValue => Opening and motion sensors

35faebb

[Fix] : Front => Debug search option, device page

375d469

Update tests

c3a35ea

Prettier

15921c1

Update tests

6aee7e2

Prettier

dee123d

Fix events

a794287

Fix tests

ab632c0

Update tests

feaeb03

Update tests

60e31db

Update test

7b3f416

Update tests

58055dc

[Fix] => Require in Manager + Tests

6caa4af

[Fix] => New tests

385f767

[Fix] => Remove only from tests

2cc148a

[Fix] => isJsonString function in separated script

bcfcf17

[Correction] => Prettier

4b366c6

[Fix] => onPortOpen/onPortData functions + tests

4b621cb

[Fix] => onPortOpen/onPortData tests

07d0e54

[Fix] => Prettier

74b93cc

[Fix] => Update tests

827ef03

[Fix] => setup tests

8e35db6

[Update] => Clean code

2077c96

[Fix] => Correction code

59cede6

[Fix] => Prettier on front

7499354

[Fix] => Setup tests

f7a4b01

billonalex force-pushed the master branch from 0de2e06 to f7a4b01 Compare December 2, 2020 09:41

VonOx added the Service Development label Dec 4, 2020

Pierre-Gilles closed this Oct 11, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Arduino service #812

Arduino service #812

billonalex commented May 20, 2020 •

edited

codecov bot commented May 27, 2020 •

edited

VonOx commented May 27, 2020

billonalex commented May 27, 2020

VonOx commented May 27, 2020

billonalex commented May 27, 2020

Pierre-Gilles commented Jun 2, 2020

billonalex commented Jun 2, 2020

Pierre-Gilles commented Jun 2, 2020

billonalex commented Jun 2, 2020

Pierre-Gilles commented Jun 2, 2020

Pierre-Gilles left a comment

Pierre-Gilles Jun 8, 2020

billonalex Jun 8, 2020

Pierre-Gilles Jun 16, 2020

billonalex commented Jun 8, 2020

billonalex commented Jun 10, 2020

Pierre-Gilles left a comment

Pierre-Gilles commented Nov 27, 2020

Arduino service #812

Arduino service #812

Conversation

billonalex commented May 20, 2020 • edited

Pull Request check-list

Description of change

codecov bot commented May 27, 2020 • edited

Codecov Report

VonOx commented May 27, 2020

billonalex commented May 27, 2020

VonOx commented May 27, 2020

billonalex commented May 27, 2020

Pierre-Gilles commented Jun 2, 2020

billonalex commented Jun 2, 2020

Pierre-Gilles commented Jun 2, 2020

billonalex commented Jun 2, 2020

Pierre-Gilles commented Jun 2, 2020

Pierre-Gilles left a comment

Choose a reason for hiding this comment

Pierre-Gilles Jun 8, 2020

Choose a reason for hiding this comment

billonalex Jun 8, 2020

Choose a reason for hiding this comment

Pierre-Gilles Jun 16, 2020

Choose a reason for hiding this comment

billonalex commented Jun 8, 2020

billonalex commented Jun 10, 2020

Pierre-Gilles left a comment

Choose a reason for hiding this comment

Pierre-Gilles commented Nov 27, 2020

billonalex commented May 20, 2020 •

edited

codecov bot commented May 27, 2020 •

edited